-
Notifications
You must be signed in to change notification settings - Fork 197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for loading DCF configuration to remote node #427
Add support for loading DCF configuration to remote node #427
Conversation
canopen/node/remote.py
Outdated
for obj in self.object_dictionary.values(): | ||
if isinstance(obj, ODRecord) or isinstance(obj, ODArray): | ||
if "PDO" in obj.name: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps it is more safe to use the indices rather than the name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean explicitly ignoring objects with indices within supported PDO ranges, i.e: 0x1400 <= obj.index < 0x1C00
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes exactly. I'm thinking using names might yield both false positives and false negatives.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks that makes sense.
I've also removed excluding objects with "COB-ID" keyword. They are being applied OK by __load_configuration_helper()
and would have been incorrectly ignored in previous commit.
c406d3e
to
b9534aa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall, just a couple of style nit-picks.
canopen/node/remote.py
Outdated
if isinstance(obj, ODRecord) or isinstance(obj, ODArray): | ||
if 0x1400 <= obj.index < 0x1c00: | ||
# Ignore PDO related objects | ||
logger.debug(f"{obj.index:#06x}: {obj.name} already applied by pdo.save()") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logger.debug(f"{obj.index:#06x}: {obj.name} already applied by pdo.save()") | |
logger.debug("0x%04X: %s already applied by pdo.save()", obj.index, obj.name) |
See #443. Do we really need this much debug logging though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, it is something that would be logged every time so it may not add much value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough. Removing.
canopen/node/remote.py
Outdated
if isinstance(obj, ODRecord) or isinstance(obj, ODArray): | ||
if 0x1400 <= obj.index < 0x1c00: | ||
# Ignore PDO related objects | ||
logger.debug(f"{obj.index:#06x}: {obj.name} already applied by pdo.save()") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough. Removing.
8bc8bbc
to
a18acac
Compare
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #427 +/- ##
==========================================
- Coverage 67.85% 67.74% -0.11%
==========================================
Files 26 26
Lines 3108 3113 +5
Branches 525 527 +2
==========================================
Hits 2109 2109
- Misses 859 864 +5
Partials 140 140
|
As part of instrument commissioning process, I want to apply the configuration values stored in a DCF to a remote node, including PDO mapping and communications parameters. If a value was not specified, then the EDS default value is used instead.
This commit provides this capability.
Potentially addresses #274.